fix: strip governance model prefix from raw body in Anthropic passthrough#1866
Open
Edward-Upton wants to merge 1 commit intomaximhq:mainfrom
Open
fix: strip governance model prefix from raw body in Anthropic passthrough#1866Edward-Upton wants to merge 1 commit intomaximhq:mainfrom
Edward-Upton wants to merge 1 commit intomaximhq:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds model prefix stripping functionality for Anthropic HTTP request bodies during passthrough handling and updates the request routing logic to ensure body modifications from pre-processing callbacks are captured and forwarded upstream to Bifrost. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/integrations/anthropic.go (1)
277-301: Consider logging on marshal failure for debuggability.The function correctly handles the model prefix stripping. However, if
sonic.Unmarshalsucceeds butsonic.Marshalfails (line 297-299), the request proceeds with the prefixed model name still in the raw body, potentially causing the original 404 issue. While this edge case is rare, a debug log would help diagnose such issues.💡 Optional: Add debug logging on marshal failure
payload["model"] = bareModel newBody, err := sonic.Marshal(payload) if err != nil { + // Note: Request will proceed with prefixed model in raw body return } ctx.Request.SetBody(newBody) }Alternatively, if a logger is accessible in this context, consider logging a warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` around lines 277 - 301, stripModelPrefixFromBody currently swallows errors from sonic.Marshal, so when Marshal fails the request keeps the prefixed model and can 404; update stripModelPrefixFromBody to log the marshal error (include the error text and context: prefixedModel, bareModel, and maybe request id from ctx if available) before returning, using the package's existing logger if accessible, otherwise use a debug/warn-friendly logging helper; ensure the log call is placed right where sonic.Marshal fails (after the err check) and do not change the current control flow beyond adding the log.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Around line 277-301: stripModelPrefixFromBody currently swallows errors from
sonic.Marshal, so when Marshal fails the request keeps the prefixed model and
can 404; update stripModelPrefixFromBody to log the marshal error (include the
error text and context: prefixedModel, bareModel, and maybe request id from ctx
if available) before returning, using the package's existing logger if
accessible, otherwise use a debug/warn-friendly logging helper; ensure the log
call is placed right where sonic.Marshal fails (after the err check) and do not
change the current control flow beyond adding the log.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
transports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/integrations/router.go
4 tasks
…ough The governance plugin's load balancer prepends the provider name to the model field in the request body (e.g. "anthropic/claude-sonnet-4-6") for internal routing. checkAnthropicPassthrough correctly strips this prefix from the parsed struct, but the raw HTTP body—used by passthrough mode to forward requests directly to Anthropic—still contained the prefixed model name. This caused 404 errors from Anthropic's API (particularly visible on count_tokens requests). Fix: strip the provider prefix from the raw body in checkAnthropicPassthrough, and re-read the body in the router after PreCallback runs so the passthrough path gets the corrected model. Made-with: Cursor
Edward-Upton
force-pushed
the
ed/fix-count-tokens-model-prefix
branch
from
c5ebf4f to
1bddf52
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
count_tokens(and other) requests when the governance plugin is active with passthrough modeanthropic/to the model name in the request body for routing, butcheckAnthropicPassthroughonly stripped the prefix from the parsed struct—not from the raw HTTP body used by passthrough mode"anthropic/claude-sonnet-4-6") → 404Changes
anthropic.go: AddedstripModelPrefixFromBody()using sonic's lazy AST (ast.NewRaw+root.Get+root.Set+root.MarshalJSON) to replace the governance-prefixed model with the bare model name. Only the"model"field is parsed and re-serialized; the rest of the body is copied as raw bytes — avoiding a fullUnmarshal/Marshalround-trip on potentially large LLM request bodies.router.go: ChangedSetRawRequestBody(rawBody)toSetRawRequestBody(ctx.Request.Body())so the passthrough path picks up the body corrected byPreCallbackinstead of the stale pre-callback snapshot.anthropic_test.go: Added 7 unit tests forstripModelPrefixFromBodycovering: prefix stripping, field preservation, non-matching model, missing model field, empty body, invalid JSON, and nested content safety (model name appearing inside message content is not modified).Conditions for the bug
All three must be true simultaneously:
loadBalanceProviderwhich adds provider prefix)BifrostContextKeyUseRawRequestBody = true)/anthropic/v1/messages/count_tokens, etc.)Test plan
go build ./bifrost-http/integrations/...)stripModelPrefixFromBody(7 cases, all passing)go test ./bifrost-http/integrations/...)count_tokensendpoint through governance + passthroughresponsesandresponses_streamrequests still work with passthroughMade with Cursor